cpp: model Protocol Buffers parse/serialize taint flow by kumarak · Pull Request #22448 · github/codeql

kumarak · 2026-08-27T15:30:42Z

Add flow summaries for the protobuf C++ API on google::protobuf::MessageLite (subtypes=true, so Message and all generated messages are covered):

ParseFrom*/MergeFrom* (string, array, Cord, istream, zero-copy and coded-stream forms) propagate taint from the encoded input to the message.
SerializeTo*/AppendTo* propagate taint from the message to the output buffer or stream; SerializeAs*/AppendTo* to the return value.
File-descriptor variants are omitted (the fd is an int, not a buffer).

Copilot

Pull request overview

Adds C++ taint-flow summaries for Protocol Buffers MessageLite APIs and inherited generated message types.

Changes:

Models parse/merge and serialization flows.
Adds representative flow tests and expected results.
Documents the analysis improvement.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
`cpp/ql/lib/ext/Protobuf.model.yml`	Defines protobuf flow summaries.
`cpp/ql/test/library-tests/dataflow/external-models/protobuf.cpp`	Adds protobuf test fixtures.
`cpp/ql/test/library-tests/dataflow/external-models/flow.expected`	Updates flow expectations.
`cpp/ql/test/library-tests/dataflow/external-models/steps.expected`	Updates summary-step expectations.
`cpp/ql/lib/change-notes/2026-08-27-protobuf-models.md`	Records the new models.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+      - ["google::protobuf", "MessageLite", True, "ParseFromZeroCopyStream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
+      - ["google::protobuf", "MessageLite", True, "ParsePartialFromZeroCopyStream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
+      - ["google::protobuf", "MessageLite", True, "ParseFromBoundedZeroCopyStream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
+      - ["google::protobuf", "MessageLite", True, "ParsePartialFromBoundedZeroCopyStream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
+      - ["google::protobuf", "MessageLite", True, "MergeFromBoundedZeroCopyStream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]


jketema

Thanks for this. I made a brief first pass over this, which should hopefully put you on the right path.

jketema

Some further comments. I think the .yml file looks good now. I would still significantly reduce the number of comments, which don't seem to add much.

jketema · 2026-09-03T09:54:16Z

cpp/ql/test/library-tests/dataflow/taint-tests/test_mad-signatures.ql fails. Otherwise this LGTM. I'll run some more internal testing.

jketema · 2026-09-03T18:36:23Z

Internal testing showed nothing out of the ordinary. So if you fix the test, then this can be merged.

jketema

LGTM. Going to fix the broken test afterwards to just get this in. Thanks for the contribution!

jketema · 2026-09-04T15:04:13Z

Going to fix the broken test afterwards to just get this in. Thanks for the contribution!

This apparently conflicts with the other PR of you that I just merged, and as don't have permission to push to your branch, I cannot resolve this myself.

Add flow summaries for the protobuf C++ API on google::protobuf::MessageLite (subtypes=true, so Message and all generated messages are covered): - ParseFrom*/MergeFrom* (string, array, Cord, istream, zero-copy and coded-stream forms) propagate taint from the encoded input to the message. - SerializeTo*/AppendTo* propagate taint from the message to the output buffer or stream; SerializeAs*/... to the return value. File-descriptor variants are omitted (the fd is an int, not a buffer).

Co-authored-by: Jeroen Ketema <93738568+jketema@users.noreply.github.com>

Replace the step-coverage function with one sink test per model row using a template source, declare the Cord overloads of the ToString methods in the stub, drop the incorrect istream comment from the fixture, and shorten the model-file and change-note comments per review.

jketema · 2026-09-11T19:08:05Z

+	template<class charT, class traits = char_traits<charT>, class Allocator = allocator<charT> >
+	class basic_string {
+	public:
+		basic_string();
+		basic_string(const charT* s, const Allocator& a = Allocator());
+		const charT* data() const;
+		size_t size() const;
+	};
+
+	typedef basic_string<char> string;


This conflicts with the definition in cpp/ql/test/library-tests/dataflow/external-models/asio_streams.cpp. This will need to be factored out into a header file, as this currently causes a test failure.

Thanks @jketema for the review. Moved the shared definition to the header file to avoid test failure.

jketema · 2026-09-11T19:09:28Z

cpp/ql/test/library-tests/dataflow/taint-tests/test_mad-signatures.ql fails.

Looks like this hasn't been updated. The test still fails.

Share standard string stubs between the protobuf and Asio fixtures and regenerate expectations for their new source locations. Add the eight missing protobuf signature expectations.

kumarak requested a review from a team as a code owner August 27, 2026 15:30

Copilot AI balanced review requested due to automatic review settings August 27, 2026 15:30

Copilot started reviewing on behalf of kumarak August 27, 2026 15:31 View session

Copilot AI reviewed Aug 27, 2026

View reviewed changes

jketema requested changes Aug 28, 2026

View reviewed changes

Comment thread cpp/ql/lib/ext/Protobuf.model.yml Outdated

Comment thread cpp/ql/test/library-tests/dataflow/external-models/protobuf.cpp Outdated

Comment thread cpp/ql/lib/change-notes/2026-08-27-protobuf-models.md Outdated

github-actions Bot added C++ documentation labels Aug 28, 2026

jketema reviewed Aug 31, 2026

View reviewed changes

jketema reviewed Sep 4, 2026

View reviewed changes

jketema previously approved these changes Sep 4, 2026

View reviewed changes

kumarak dismissed jketema’s stale review via 938e041 September 9, 2026 05:02

kumarak force-pushed the kumarak/cpp-protobuf-flow-models branch from 49ffde7 to 938e041 Compare September 9, 2026 05:02

kumarak and others added 4 commits September 9, 2026 12:18

Update cpp/ql/lib/change-notes/2026-08-27-protobuf-models.md

a7e2584

Co-authored-by: Jeroen Ketema <93738568+jketema@users.noreply.github.com>

fix(cpp): correct protobuf string parse models and expand test coverage

d84a302

kumarak force-pushed the kumarak/cpp-protobuf-flow-models branch from 938e041 to d603cdd Compare September 9, 2026 16:35

kumarak requested a review from jketema September 9, 2026 16:35

jketema reviewed Sep 11, 2026

View reviewed changes

test(cpp): fix protobuf model CI failures

d293e87

Share standard string stubs between the protobuf and Asio fixtures and regenerate expectations for their new source locations. Add the eight missing protobuf signature expectations.

Uh oh!

Conversation

kumarak commented Aug 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

jketema left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jketema left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jketema commented Sep 3, 2026

Uh oh!

jketema commented Sep 3, 2026

Uh oh!

jketema left a comment

Choose a reason for hiding this comment

Uh oh!

jketema commented Sep 4, 2026

Uh oh!

jketema Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kumarak Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

jketema commented Sep 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jketema Sep 11, 2026 •

edited

Loading